Fix build for various targets #12
Conversation
📝 WalkthroughWalkthroughWiden CI target matrix and replace Rust toolchain setup; bump package version; add platform-specific errno get/set for multiple OSes; change several integer math functions to return Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @Cargo.toml:
- Around line 1-8: The Cargo.toml version was increased only as a patch (0.1.2
-> 0.1.3) but this PR introduces public API changes (e.g., modified function
signatures in src/math/integer.rs), so update the package version to a minor
bump (0.2.0) instead of a patch to reflect the breaking change; edit the version
field in Cargo.toml from "0.1.3" to "0.2.0" and ensure any related changelog or
documentation is updated to indicate the API change.
🧹 Nitpick comments (3)
src/math/bigint.rs (1)
107-117: Let-chains are fine, but confirm MSRV (and considercrate::mul_addin fallback math)
- The
if let Some(x) = ... && x.is_finite()simplification looks good, but it does assume let-chains support; please confirm the crate’s intended MSRV/toolchain baseline.- In
log_bigint/log10_bigintfallback paths, consider usingcrate::mul_add(a, b, c)forc + a*bto match project guidance on FMA-sensitive platforms (as per coding guidelines).Possible `mul_add` tweak (fallback path)
- let log_n = crate::m::log(x) + std::f64::consts::LN_2 * (e as f64); + let log_n = crate::mul_add(std::f64::consts::LN_2, e as f64, crate::m::log(x)); @@ - Ok(crate::m::log10(x) + std::f64::consts::LOG10_2 * (e as f64)) + Ok(crate::mul_add(std::f64::consts::LOG10_2, e as f64, crate::m::log10(x)))Also applies to: 138-148, 159-169
.github/workflows/rust.yml (1)
60-102: Exotic target checks: solid coverage; consider consolidating apt installs (optional)The target list is great. Optional: consolidate
apt-get update+ installs into fewer steps to reduce time/flakiness.src/math/integer.rs (1)
780-855: Tests should assert the specific error type (EDOM), not just “some error”Right now the Python-error branches only check
rs.is_err(). Per learnings, tests should validate error types (ValueError →Error::EDOM, OverflowError →Error::ERANGE). Here, the expected error for negative inputs isError::EDOM. Based on learnings, tests should be tightened accordingly.Example tightening (one pattern)
- Err(_) => { - assert!(rs.is_err(), "isqrt({n}): py raised error but rs={rs:?}"); - } + Err(_) => { + assert_eq!(rs, Err(crate::Error::EDOM), "isqrt({n}): py raised error but rs={rs:?}"); + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/rust.ymlCargo.tomlsrc/err.rssrc/math/bigint.rssrc/math/integer.rs
🧰 Additional context used
📓 Path-based instructions (2)
src/math/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/math/**/*.rs: Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Files:
src/math/bigint.rssrc/math/integer.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/math/bigint.rssrc/err.rssrc/math/integer.rs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
src/math/bigint.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Applied to files:
src/math/bigint.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Applied to files:
src/math/bigint.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/math/bigint.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Applied to files:
src/math/bigint.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Applied to files:
src/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Applied to files:
src/math/integer.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (macos-latest)
- GitHub Check: Build and test (windows-latest)
- GitHub Check: Check exotic targets
🔇 Additional comments (9)
src/err.rs (1)
24-55: Android errno symbol (libc::__errno) needs a quick compile confirmationThe cfg split and excluding Android from the unix fallback looks right, but please confirm
libc::__errno()is available for your Android targets/toolchain (and returns a writable errno pointer as expected). This is a likely build-break point if the symbol differs across libc/bionic variations.Also applies to: 58-89
.github/workflows/rust.yml (4)
30-36: Python 3.14 in CI: confirm availability on all runners
actions/setup-python@v5withpython-version: "3.14"may be fine, but it’s worth confirming it resolves consistently acrossubuntu-latest,windows-latest, andmacos-latestto avoid transient CI failures.
34-36: Toolchain action swap looks goodSwitching to
dtolnay/rust-toolchain@stableis a clean simplification.
45-59: macOS cross-targetcargo checkadditions are a good fit for “build for various targets”The Intel macOS + iOS target checks provide early signal without needing full link steps.
103-112: Clippy with-Dwarningsis a nice guardrailThis should help keep cross-platform portability changes tidy.
src/math/integer.rs (4)
81-89:isqrtEDOM on negative input is consistent with crate error modelThis aligns with the
crate::Result/crate::Error::EDOMconvention.
234-253:factorialsignature + EDOM handling look consistentNo issues with the updated return type and negative guard.
653-678:combnow returns typed EDOM errors—goodGuarding
n < 0 || k < 0withErr(crate::Error::EDOM)matches the documented behavior.
680-710:permnegative-k handling is correctThe
Some(k) if k < 0 => Err(EDOM)case is a good catch.
| [package] | ||
| name = "pymath" | ||
| author = "Jeong, YunWon <[email protected]>" | ||
| authors = ["Jeong, YunWon <[email protected]>"] | ||
| repository = "https://github.com/RustPython/pymath" | ||
| description = "A binary representation compatible Rust implementation of Python's math library." | ||
| version = "0.1.2" | ||
| version = "0.1.3" | ||
| edition = "2024" | ||
| license = "PSF-2.0" |
There was a problem hiding this comment.
Patch version bump likely mismatches breaking public API changes elsewhere in the PR
0.1.2 -> 0.1.3 is a patch bump, but this PR changes public function signatures (e.g., src/math/integer.rs return types). If this crate is consumed externally, consider bumping the “minor” (0.2.0) instead to avoid surprising downstream breakage.
Proposed change
[package]
name = "pymath"
authors = ["Jeong, YunWon <[email protected]>"]
repository = "https://github.com/RustPython/pymath"
description = "A binary representation compatible Rust implementation of Python's math library."
-version = "0.1.3"
+version = "0.2.0"
edition = "2024"
license = "PSF-2.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [package] | |
| name = "pymath" | |
| author = "Jeong, YunWon <[email protected]>" | |
| authors = ["Jeong, YunWon <[email protected]>"] | |
| repository = "https://github.com/RustPython/pymath" | |
| description = "A binary representation compatible Rust implementation of Python's math library." | |
| version = "0.1.2" | |
| version = "0.1.3" | |
| edition = "2024" | |
| license = "PSF-2.0" | |
| [package] | |
| name = "pymath" | |
| authors = ["Jeong, YunWon <[email protected]>"] | |
| repository = "https://github.com/RustPython/pymath" | |
| description = "A binary representation compatible Rust implementation of Python's math library." | |
| version = "0.2.0" | |
| edition = "2024" | |
| license = "PSF-2.0" |
🤖 Prompt for AI Agents
In @Cargo.toml around lines 1 - 8, The Cargo.toml version was increased only as
a patch (0.1.2 -> 0.1.3) but this PR introduces public API changes (e.g.,
modified function signatures in src/math/integer.rs), so update the package
version to a minor bump (0.2.0) instead of a patch to reflect the breaking
change; edit the version field in Cargo.toml from "0.1.3" to "0.2.0" and ensure
any related changelog or documentation is updated to indicate the API change.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/err.rs (1)
3-20: Portability: defineError::{EDOM,ERANGE}fromlibc::{EDOM,ERANGE}(and add#[repr(c_int)]).
Right nowErroris hardcoded to 33/34 (Line 6-8). With the new guarded comparisons (Line 17-18), any platform where those values differ will mis-map errno.Proposed fix
use std::ffi::c_int; // The values are defined in libc #[derive(Debug, PartialEq, Eq)] +#[repr(c_int)] pub enum Error { - EDOM = 33, - ERANGE = 34, + EDOM = libc::EDOM, + ERANGE = libc::ERANGE, }.github/workflows/rust.yml (1)
29-44: Remove unusedmatrix.rustvariable and note Python 3.14 won't be pre-cached.Line 20 defines
rust: [stable]in the matrix, but it's never referenced in the workflow steps, so it should be removed. Additionally, Python 3.14 is not yet pre-cached in GitHub Actions hosted runners as of January 2026—actions/setup-python@v5will download and install it on-the-fly, which adds unnecessary setup time. Consider using a Python version that's in the toolcache (e.g., 3.12), or if 3.14 is required, accept the slower installation step.
🤖 Fix all issues with AI agents
In @src/math/integer.rs:
- Around line 81-89: The isqrt implementation now returns
Err(crate::Error::EDOM) for negative inputs; update the tests (e.g.,
test_isqrt_impl) to assert the specific error variant rather than only checking
rs.is_err(), by invoking isqrt(-1) (or equivalent negative BigInt) and asserting
it equals Err(crate::Error::EDOM) or matches Error::EDOM; ensure you reference
the isqrt function and the crate::Error::EDOM variant in the assertion so
regressions to other error kinds (like ERANGE) will fail the test.
🧹 Nitpick comments (3)
.github/workflows/rust.yml (2)
45-59: macOS cross-target checks look reasonable; ensuremul_addfeature is intended for all macOS target checks.
Because these checks reuse${{ matrix.features }}, macOS will alsocargo checkiOS withmul_addenabled (Line 51-58). Ifmul_addis meant to be macOS-host-only, consider splitting features for host vs cross-target checks.
103-112: Clippy job is good; consider linting the macOS-onlymul_addfeature too.
Right now clippy runscomplex,malachite-bigintonly (Line 111-112), so warnings behindmul_addmay be missed.src/err.rs (1)
24-54: Minor: consider deduplicating errno pointer retrieval to avoid drift betweenset_errnoandget_errno.
Today, the per-target selection logic is duplicated; a smallerrno_ptr()helper per platform would reduce the chance of future mismatches.Also applies to: 56-88
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/rust.ymlCargo.tomlsrc/cmath/misc.rssrc/err.rssrc/math/bigint.rssrc/math/integer.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- src/math/bigint.rs
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/cmath/misc.rssrc/err.rssrc/math/integer.rs
src/cmath/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/cmath/**/*.rs: For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Files:
src/cmath/misc.rs
src/math/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/math/**/*.rs: Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Files:
src/math/integer.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/cmath/misc.rssrc/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Applied to files:
src/cmath/misc.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
src/cmath/misc.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Applied to files:
src/cmath/misc.rssrc/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Applied to files:
src/cmath/misc.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Applied to files:
src/cmath/misc.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Applied to files:
src/cmath/misc.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Applied to files:
src/cmath/misc.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/cmath/misc.rssrc/err.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Applied to files:
src/cmath/misc.rssrc/math/integer.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (macos-latest)
- GitHub Check: Build and test (windows-latest)
- GitHub Check: Check exotic targets
🔇 Additional comments (7)
.github/workflows/rust.yml (1)
60-102: Exotic targets: Android check may need an NDK/linker depending on build scripts—verify.
cargo check --target aarch64-linux-android(Line 76-79) can still fail if any dependency/build script needs a target linker or C toolchain; you currently don’t install an NDK. Also consider doingapt-get updatebefore later installs (Line 85, 93) for resilience.src/cmath/misc.rs (1)
20-28:phase()errno mapping change is fine, but it relies onError::EDOM/ERANGEmatching platform errno.
The guard comparisons (Line 25-26) are only as portable as theErrordiscriminants; ensureError::{EDOM,ERANGE}are defined from platform libc constants (see src/err.rs). As per coding guidelines, good use ofset_errno(0)before the libm call.src/math/integer.rs (3)
234-253:factorial: return type + EDOM on negative input looks correct.
The early return onn < 0(Line 240-242) matches the new error model.
653-678:comb: EDOM on negative inputs + 0 whenk > nmatches CPython semantics.
The guard at Line 659-661 is clear and consistent with the crate-wide error handling.
680-710:perm: negative handling is correct (includingSome(k) if k < 0).
The match arm at Line 694-696 cleanly covers negativekwithout extra branches.src/err.rs (2)
90-108:is_error()mapping looks consistent with existing underflow behavior.
The ERANGE “underflow to ~0” exception (Line 97-106) is preserved and the guarded comparisons align with the refactor.
24-54: FFI syntaxsafe fnin extern blocks is stable and supported—no changes needed.The
safe fn _errno()declaration insideextern "C"blocks is supported on stable Rust since version 1.82 (October 2024). The code correctly usesset_errno(0)to clear errno before libm calls, which aligns with proper error handling practices. The project has no MSRV constraint that would prevent this syntax, and there are no compilation concerns on Windows.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/err.rs (1)
59-88: Platform-specific errno retrieval looks correct.The platform-specific implementations match the set_errno pattern correctly.
♻️ Optional: Deduplicate Windows FFI declaration
The
_errno()FFI declaration is duplicated in bothset_errnoandget_errno. Consider extracting it to a module-level declaration:+#[cfg(target_os = "windows")] +unsafe extern "C" { + safe fn _errno() -> *mut i32; +} + /// Set errno to the given value. #[inline] pub(crate) fn set_errno(value: i32) { ... #[cfg(target_os = "windows")] unsafe { - unsafe extern "C" { - safe fn _errno() -> *mut i32; - } *_errno() = value; } ... } /// Get the current errno value. #[inline] pub(crate) fn get_errno() -> i32 { ... #[cfg(target_os = "windows")] unsafe { - unsafe extern "C" { - safe fn _errno() -> *mut i32; - } *_errno() } ... }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/rust.ymlCargo.tomlsrc/cmath/misc.rssrc/err.rssrc/math/bigint.rssrc/math/integer.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cmath/misc.rs
🧰 Additional context used
📓 Path-based instructions (2)
src/math/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/math/**/*.rs: Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Files:
src/math/bigint.rssrc/math/integer.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/math/bigint.rssrc/err.rssrc/math/integer.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
src/math/bigint.rssrc/err.rsCargo.tomlsrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/math/bigint.rssrc/err.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Applied to files:
src/math/bigint.rsCargo.toml
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Applied to files:
src/math/bigint.rsCargo.tomlsrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Applied to files:
src/math/bigint.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Applied to files:
src/math/bigint.rsCargo.toml
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Applied to files:
src/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Applied to files:
src/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Applied to files:
src/math/integer.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (macos-latest)
- GitHub Check: Build and test (windows-latest)
- GitHub Check: Check exotic targets
🔇 Additional comments (15)
Cargo.toml (1)
3-3: LGTM! Version bump and metadata update.The version increment to 0.1.3 and the authors field addition are appropriate for this release.
Also applies to: 6-6
src/math/bigint.rs (3)
113-117: LGTM! Clean refactoring using let-chains.The combined conditional
if let Some(x) = n.to_f64() && x.is_finite()is more concise and idiomatic than the previous two-step pattern. The logic and behavior remain unchanged.
144-148: LGTM! Consistent with log_bigint refactoring.Same improvement as in log_bigint using let-chains.
165-169: LGTM! Consistent with other log functions.Same improvement as in log_bigint and log2_bigint using let-chains.
.github/workflows/rust.yml (4)
35-35: LGTM! Modernized Rust toolchain setup.Switching from
actions-rs/toolchain@v1(which is deprecated) todtolnay/rust-toolchain@stableis the recommended approach.
45-58: LGTM! macOS cross-compilation validation.Adding checks for Intel macOS (x86_64-apple-darwin) and iOS (aarch64-apple-ios) targets ensures compatibility across Apple platforms.
60-113: LGTM! Comprehensive exotic target coverage.The new job validates builds across diverse platforms (32-bit Linux, Android, ARM64, musl, FreeBSD, WASM), directly addressing the PR objective to fix builds for various targets.
115-124: LGTM! Clippy enforcement.Adding a dedicated clippy job with
-Dwarningshelps maintain code quality.src/math/integer.rs (4)
83-88: LGTM! Proper error handling migration.The migration from
Result<BigInt, ()>tocrate::Result<BigInt>withErr(crate::Error::EDOM)for negative inputs is correct and aligns with the coding guidelines. As per coding guidelines, returningResultdirectly withErr(crate::Error::EDOM)is the recommended approach.
236-241: LGTM! Consistent error handling for factorial.The error handling update matches the pattern in isqrt, correctly using
EDOMfor negative input domain errors. As per coding guidelines, this is the correct approach.
655-660: LGTM! Consistent error handling for comb.The error handling update correctly uses
EDOMfor domain errors (negative n or k). As per coding guidelines, this is the correct approach.
682-694: LGTM! Consistent error handling for perm.The error handling update correctly uses
EDOMfor both negative n and negative k cases. As per coding guidelines, this is the correct approach.src/err.rs (3)
17-18: LGTM! Guards ensure consistency with enum values.Using guards (
x if x == Error::EDOM as c_int) instead of hardcoded literals ensures the match arms stay synchronized with the enum discriminants.
95-96: LGTM! Guards replace hardcoded errno constants.Using guards (
e if e == Error::EDOM as i32) is consistent with the TryFrom implementation and more maintainable than hardcoded libc constants.
27-54: Platform-specific errno handling is correct.The implementations for all platforms are appropriate: Linux via
__errno_location(), Android via__errno(), macOS and BSD variants via__error(), Windows via_errno(), and WASM as a no-op. Allset_errnocalls in the codebase correctly pass0to clear errno before libm calls, and error reporting usesResultwithErr(Error::EDOM)orErr(Error::ERANGE)as required. Tests properly verify both correct return values and error type conversions (ValueError → EDOM, OverflowError → ERANGE).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/err.rs (2)
31-54: iOS target is not explicitly handled in set_errno.iOS (
target_os = "ios") will fall through to the generic Unix branch usinglibc::__error(), which should work since iOS uses the same function as macOS. However, for clarity and to avoid potential issues, consider adding explicit iOS handling.Suggested fix to add explicit iOS handling
#[cfg(target_os = "macos")] unsafe { *libc::__error() = value; } + #[cfg(target_os = "ios")] + unsafe { + *libc::__error() = value; + } #[cfg(target_os = "windows")] unsafe { *_errno() = value; } - #[cfg(all(unix, not(any(target_os = "linux", target_os = "android", target_os = "macos"))))] + #[cfg(all(unix, not(any(target_os = "linux", target_os = "android", target_os = "macos", target_os = "ios"))))] unsafe { // FreeBSD, NetBSD, OpenBSD, etc. use __error() *libc::__error() = value; }
60-85: Same iOS handling consideration applies to get_errno.The get_errno function has the same pattern as set_errno. iOS will work via the generic Unix fallback, but explicit handling would be clearer.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/rust.ymlCargo.tomlsrc/cmath/misc.rssrc/err.rssrc/math/bigint.rssrc/math/integer.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/math/bigint.rs
- src/cmath/misc.rs
- Cargo.toml
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/err.rssrc/math/integer.rs
src/math/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/math/**/*.rs: Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Files:
src/math/integer.rs
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Applied to files:
src/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
src/err.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/err.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Applied to files:
src/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Applied to files:
src/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Applied to files:
src/math/integer.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (windows-latest)
- GitHub Check: Check exotic targets
- GitHub Check: Build and test (macos-latest)
🔇 Additional comments (13)
.github/workflows/rust.yml (4)
34-35: Good choice switching to dtolnay/rust-toolchain.The
dtolnay/rust-toolchainaction is actively maintained and simpler to use than the deprecatedactions-rs/toolchain.
44-58: macOS cross-compilation checks look correct.The conditional steps for Intel macOS (
x86_64-apple-darwin) and iOS (aarch64-apple-ios) are properly gated withrunner.os == 'macOS'.
60-113: Comprehensive exotic targets coverage.The job covers a good variety of targets including 32-bit Linux, Android, ARM64, musl, FreeBSD, and WASM variants. Using
cargo checkinstead ofcargo buildis appropriate for cross-compilation validation where you may not have the full toolchain.
115-124: Clippy job configured correctly.Using
-Dwarningsensures the build fails on any Clippy warnings, which is a good practice for maintaining code quality.src/err.rs (3)
88-107: is_error implementation follows coding guidelines correctly.The function properly checks errno after libm calls and returns
ResultwithError::EDOMorError::ERANGE. The underflow-to-zero handling (threshold < 1.5) matches CPython's behavior per bpo-46018.
3-6:safe fn _errno()syntax is valid and properly supported.The
safe fndeclaration inside theunsafe extern "C"block is valid Rust syntax (RFC 3484, stabilized in Rust 1.82). Since the project targets edition 2024, this syntax is fully supported. The Windows errno pointer retrieval is correct.
8-12: Hardcoded errno values EDOM=33 and ERANGE=34 are universally correct across all platforms.These values are standardized across POSIX systems (Linux, macOS, BSD variants, Android) and Windows. The codebase appropriately handles platform-specific errno access mechanisms via
get_errno()andset_errno(), but the numeric constants themselves are invariant and require no exotic target verification.src/math/integer.rs (6)
83-89: isqrt error handling follows coding guidelines correctly.Returns
Err(crate::Error::EDOM)for negative input, which aligns with the coding guidelines for error handling.
236-253: factorial error handling is correct.Properly returns
Err(crate::Error::EDOM)for negative input, matching Python'sValueErrorbehavior.
655-678: comb implementation is correct.
- Returns
Err(crate::Error::EDOM)for negative n or k- Returns
Ok(BigUint::zero())when k > n (correct per combinatorics definition)- Uses symmetry optimization
k.min(n - k)for efficiency
688-697: perm properly handles negative k in Option.The pattern matching correctly handles the
Some(k) if k < 0case to returnErr(crate::Error::EDOM).
712-856: Test functions comprehensively validate against Python.The tests properly:
- Compare Rust results with Python's
mathmodule- Check both
Okresults and error cases- Use edge values to cover boundary conditions
1063-1133: Proptest coverage is thorough.The property-based tests cover:
- Small and large value ranges
- Negative inputs for error cases
- Various combinations for gcd/lcm/comb/perm
This provides good confidence in correctness across a wide input space.
Summary by CodeRabbit
Chores
Bug Fixes
Compatibility
✏️ Tip: You can customize this high-level summary in your review settings.